-
Notifications
You must be signed in to change notification settings - Fork 415
NocCostHandler class #2724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NocCostHandler class #2724
Conversation
@soheilshahrouz : please resolve conflicts / rebase so I can review just what's new in this PR. |
Also run a NoC QoR run. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @soheilshahrouz , great PR! I have left some comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good; just a few comments. Feel free to merge once CI passes and you make these commenting changes.
This function clears a BlkLocRegistry object. It makes more sense if it is implemented as a BlkLocRegistry method
Synthetic benchmarks with 16 or more NoC routers
|
Looks good to me. It looks like the only CI failures are the odd/even routing unit tests, which I think you are disabling due to a known problem in them. Is this ready to merge now? |
@soheilshahrouz : ok for me to merge this? |
@vaughnbetz I ran non-odin nightly tests on wintermute and did not see any failures. I think this is ready to be merged. |
This PR adds encapsulates functions used to compute NoC cost terms into a class named
NocCostHandler
. Static variables innoc_place_utils.cpp
are converted to member variables ofNocCostHandler
.